Skip to content

fix(keychain): store large blobs on windows#474

Merged
Benehiko merged 1 commit intomainfrom
store/fix/windows
Feb 27, 2026
Merged

fix(keychain): store large blobs on windows#474
Benehiko merged 1 commit intomainfrom
store/fix/windows

Conversation

@Benehiko
Copy link
Copy Markdown
Member

Some credentials - especially auth tokens, can be quite large. On windows the wincred store only supports up to 2560 bytes in length. This solution below chunks the credential into parts and stores those parts alongside the main ID.

Chunked credentials are always skipped when filtering credentials from the store by checking the chunkCountKey from the "main" credentials metadata.

The safelyCleanMetada function removes the attribute when reading the credential from the store.

Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
@Benehiko Benehiko requested a review from joe0BAB February 27, 2026 08:24
Copy link
Copy Markdown

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Looks good! No issues found in the chunking implementation for Windows Credential Manager. The code properly handles large blobs by splitting them into chunks and reassembling them on read.

Comment on lines +376 to +387
var rawBlob []byte
if countStr, ok := gcAttributes[chunkCountKey]; ok {
count, err := strconv.Atoi(countStr)
if err != nil {
return nil, fmt.Errorf("invalid chunk count %q: %w", countStr, err)
}
rawBlob, err = k.readChunks(id, count)
if err != nil {
return nil, err
}
} else {
rawBlob = gc.CredentialBlob
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it make sense to dedupe this? Seeing the same code above.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verbosity is best here

assert.False(t, isChunkCredential([]wincred.CredentialAttribute{}))
})
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want some more tests that verify Save/Filter/Get/Delete on credentials that need to be chunked work as expected?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely, but i think this is good enough to get this fix in quickly

Copy link
Copy Markdown
Collaborator

@joe0BAB joe0BAB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tweaks are non blocking, let's do them in a follow up

@Benehiko Benehiko merged commit 73663b9 into main Feb 27, 2026
32 of 33 checks passed
@Benehiko Benehiko deleted the store/fix/windows branch February 27, 2026 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants